Add observation.metadata.choice_nodes
#4422
Conversation
tybug
force-pushed
the
observability-choice-nodes
branch
from
576db1a to
37e0960
Compare
tybug
force-pushed
the
observability-choice-nodes
branch
from
d82561e to
a02dbac
Compare
There was a problem hiding this comment.
Do we have a concrete use-case in mind for the node constraints? If not, how about b64-encoded bytes format for the choice sequence? It'd be a fair bit smaller, and it looks like the human-readable version is already complicated enough that outside parties will want to use our code to deal with that.
Either way, I was thinking that we could have a flat list of [start, end, was_discarded, label] values, derived from the Spans class to track the tree structure of the input.
| "interesting_origin": { | ||
| "type": ["string", "null"], | ||
| "description": "The internal InterestingOrigin object for failing tests, if and only if ``status == \"failed\"``. The ``traceback`` string value is derived from this object." | ||
| }, |
There was a problem hiding this comment.
IIRC we already store this in the top-level status_reason key.
[goes and checks] aaaaaannnd, we mishandle non-failing cases, oops.
There was a problem hiding this comment.
yeah, INVALID and OVERRUN get collapsed together (which is probably-good as a library agnostic format)
There was a problem hiding this comment.
sorry, I thought this was status, not status_reason. The actual reason I split this out is because hypofuzz wants the InterestingOrigin object itself, not the str(origin) version in status_reason.
[e: and this means the type here is wrong, fixing..]
[e2: nope, it is a string because we're documenting the file format here, not the in-memory format. Hmm, unfortunate that the two are similar but different...]
Concrete use case is passing in-memory to hypofuzz, mostly for ordering corpus elements, cmd+f "choice_nodes" in https://github.com/Zac-HD/hypofuzz/pull/113/files. I don't have a consumer of the serialized file format in mind, but my thinking is "if you're in a position to be using hypothesis utilities to transform the observations, you should just hook |
|
Yeah, OK, fair enough. Feels inelegant but I think moving fast at the cost of inelegant formats is the right call for now. |
tybug
force-pushed
the
observability-choice-nodes
branch
from
e6d9c62 to
6dc159d
Compare
I went with |
| # We should only be sending observability reports for datas that have finished | ||
| # being modified. | ||
| assert data.frozen | ||
|
|
||
| if status_reason is not None: |
There was a problem hiding this comment.
data needs to be frozen to access data.spans, and at that point it seems reasonable to require that all datas passed here are frozen. I don't think freezing more than we used to at any of the callers is harmful, and anyway the whole point of .freeze is a phase-transition at which point it cannot be modified, which we definitely expect if we're reporting an observation for a data.
| Hypothesis Metadata | ||
| ^^^^^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
Let's
- move the information-message schema first
- make this a subheading of the test-case schema
- consider pulling out the choice_nodes and choice_spans to a second subheading and collapsing
<details>by default (it's a lot of very-niche-interest text)
There was a problem hiding this comment.
I'm not sure sphinx has collapsible elements unless we install some third party extension 🤔
There was a problem hiding this comment.
It's a cute trick, the built-in only directive allows for arbitrary html:
.. only:: html
.. raw:: html
<details>
<summary>Click to expand</summary>
Put whatever you want here, and in the HTML build only it'll be inside a details tag!
.. only:: html
.. raw:: html
</details>There was a problem hiding this comment.
happy to collapse for now, but TBH I think observability might deserve its own page at this point. (I think ghostwriter does as well). IMO a selling point of big api reference pages is that you don't have to worry about things like niche content or collapsing text
(which also makes cmd+f harder) TIL cmd+f autoexpands <details>
but still, collapsing makes it easy to miss, and this is a pretty important section for the right subset of people
There was a problem hiding this comment.
no, good argument, leave it uncollapsed and we'll move it later.
Closes (most of) #4387
PrimitiveProvider.on_observation#4416To do:
choice_nodesformat (nonstandard nan patterns, large ints, etc)observability.OBSERVABILITY_CHOICE_NODES@settingsfor this? (as a follow-up?)I'm also combining this with changes toI've split this out and have dropped it from this PRBuildContext(for hypofuzz), so that this PR can be "the entirety of the changes necessary to make HypofuzzProvider work".